refactor: integrate paykit sdk#1040
Conversation
Greptile SummaryThis PR replaces Bitkit's hand-rolled Paykit plumbing with the published
Confidence Score: 4/5The core payment flow and session lifecycle look structurally sound; the main risks are edge cases in the new blocking-inside-synchronized SDK state store and empty contact names when the SDK returns a profile with no display data. The architectural shift is large but well-scoped: the SDK takes over state management that was previously hand-coded, and the delegation boundary is clear. The new PaykitSdkStateBlobStore uses runBlocking(ioDispatcher) inside a synchronized block — not a deadlock under normal load but a thread-starvation risk under sustained IO pressure. PaykitSdkSessionProvider.clearSessionAccess() uses a bare runBlocking {} without a dispatcher, which could misbehave if called from an unusual thread context. The backup restore path for legacy (pre-SDK) backups silently swallows SDK state-clearing errors. The contact-name-empty edge case is a UI regression when the SDK's profile record lacks both displayName and decodable extraJson. None of these are showstoppers, but the blocking-coroutine nesting deserves attention before shipping to broad audiences. PaykitSdkService.kt (the PaykitSdkStateBlobStore and PaykitSdkSessionProvider inner classes), BackupRepo.kt (legacy restore path around line 619), and PubkyRepo.kt (contactProfile method).
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/services/PaykitSdkService.kt | New singleton service wrapping the Paykit SDK; mixes runBlocking inside a synchronized block (saveStateBlobAtomically) and has a bare runBlocking in PaykitSdkSessionProvider.clearSessionAccess(). |
| app/src/main/java/to/bitkit/data/keychain/Keychain.kt | Adds a new synchronous upsert(ByteArray) method using runBlocking(this.coroutineContext); consistent with the existing snapshot pattern but called from a synchronized block, risking thread starvation under IO saturation. |
| app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt | Substantially trimmed by delegating link/handshake/recovery state to the SDK; backup snapshot now delegates to PaykitSdkService.exportBackupState(); logic looks correct. |
| app/src/main/java/to/bitkit/repositories/PubkyRepo.kt | Delegates session/profile/contact operations to PaykitSdkService; introduces contactProfileOverrides in PubkyStore and snapshotContactProfileOverrides/restoreContactProfileOverrides for backup; contact name may be empty when paykitProfile has no displayName and no extraJson. |
| app/src/main/java/to/bitkit/repositories/BackupRepo.kt | Backup listeners refactored to observeBackupChanges helper; wallet restore silently swallows SDK state-clearing errors for legacy backups (null paykitSdkBackupState). |
| app/src/main/java/to/bitkit/services/PubkyService.kt | Thin wrapper now fully delegates to PaykitSdkService; straightforward and correct. |
| gradle/libs.versions.toml | Bumps paykit-android from rc8 to rc21; no other dependency changes. |
| app/src/main/java/to/bitkit/models/BackupPayloads.kt | Replaces PrivatePaykitContactLinkBackupV1 map with a single paykitSdkBackupState string and adds pubkyContactProfileOverrides; old backup fields removed with no migration path for existing contact-link data. |
| app/src/main/java/to/bitkit/models/PubkyProfile.kt | Adapts to SDK PubkyProfile/PaykitProfile types; fromPaykitProfile may produce an empty contact name if displayName and extraJson are both absent. |
| app/src/main/java/to/bitkit/usecases/WipeWalletUseCase.kt | Wipe sequence unchanged in substance; closeAndClear() now delegates SDK state clearing, then keychain.wipe() removes all persisted state. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant App as App/UI
participant PPR as PrivatePaykitRepo
participant SDK as PaykitSdkService
participant PaykitSdk as PaykitSdk (native)
participant Keychain as Keychain
participant BR as BackupRepo
App->>PPR: prepareSavedContacts(publicKeys)
PPR->>SDK: ensureLinkWithPeer(counterparty)
SDK->>PaykitSdk: ensureLinkWithPeer()
PaykitSdk->>Keychain: saveStateBlobAtomically() [synchronized + runBlocking]
SDK->>BR: backupStateVersion++ (via withStateRevisionTracking)
PPR->>SDK: syncPrivatePaymentListsWithReservations(updates)
SDK->>PaykitSdk: syncPrivatePaymentListsWithReservationsAndProcessOutbound()
PaykitSdk->>Keychain: saveStateBlobAtomically()
SDK->>BR: backupStateVersion++
App->>PPR: beginSavedContactPayment(publicKey)
PPR->>SDK: prepareAndResolveContactPayment(counterparty)
SDK->>PaykitSdk: prepareAndResolveContactPayment()
PaykitSdk-->>SDK: ContactPaymentResolution
SDK-->>PPR: PaykitContactPaymentResolution
PPR-->>App: PublicPaykitPaymentResult
BR->>PPR: backupSnapshot()
PPR->>SDK: exportBackupState()
SDK->>PaykitSdk: exportBackupString()
PaykitSdk-->>SDK: String (opaque blob)
SDK-->>BR: paykitSdkBackupState
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant App as App/UI
participant PPR as PrivatePaykitRepo
participant SDK as PaykitSdkService
participant PaykitSdk as PaykitSdk (native)
participant Keychain as Keychain
participant BR as BackupRepo
App->>PPR: prepareSavedContacts(publicKeys)
PPR->>SDK: ensureLinkWithPeer(counterparty)
SDK->>PaykitSdk: ensureLinkWithPeer()
PaykitSdk->>Keychain: saveStateBlobAtomically() [synchronized + runBlocking]
SDK->>BR: backupStateVersion++ (via withStateRevisionTracking)
PPR->>SDK: syncPrivatePaymentListsWithReservations(updates)
SDK->>PaykitSdk: syncPrivatePaymentListsWithReservationsAndProcessOutbound()
PaykitSdk->>Keychain: saveStateBlobAtomically()
SDK->>BR: backupStateVersion++
App->>PPR: beginSavedContactPayment(publicKey)
PPR->>SDK: prepareAndResolveContactPayment(counterparty)
SDK->>PaykitSdk: prepareAndResolveContactPayment()
PaykitSdk-->>SDK: ContactPaymentResolution
SDK-->>PPR: PaykitContactPaymentResolution
PPR-->>App: PublicPaykitPaymentResult
BR->>PPR: backupSnapshot()
PPR->>SDK: exportBackupState()
SDK->>PaykitSdk: exportBackupString()
PaykitSdk-->>SDK: String (opaque blob)
SDK-->>BR: paykitSdkBackupState
Comments Outside Diff (1)
-
app/src/main/java/to/bitkit/repositories/BackupRepo.kt, line 619-628 (link)SDK state-clear failure silently ignored during legacy backup restore
When
paykitSdkBackupStateisnull(restoring a backup created before this PR),privateRepo.restoreBackup(null)is called and any failure is only logged viaonFailure { Logger.warn(...) }— execution continues regardless. InsiderestoreBackup(null),paykitSdkService.clearState()deletes thePAYKIT_SDK_STATEkeychain entry. If this deletion fails (e.g., keystore error), the stale SDK state persists while the rest of the wallet is restored from the new backup, leaving contact-link and session state out of sync with the freshly restored wallet. The successful path (paykitSdkBackupState != null) uses.getOrThrow()— the legacy path should follow the same convention or at least propagate the failure to surface the inconsistency.
Reviews (1): Last reviewed commit: "fix: preserve paykit cancellation" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8202a59774
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
For the legacy backup migration note: this is intentional for this PR. The old private Paykit link backup format never shipped, so there is no production data to migrate. Treating it as if it never existed keeps the restore path simpler. |
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
Left one inline comment.
|
That is not necessarily due to this change, because I saw it on other PR also - however e2e tests here failed partially because of this. The failure is intermittent and most of the time tests pass after re-runs. To reproduce:
Result after hitting "Continue" on the following screen: Attaching logs from e2e run where this happened: |
…tive-integration # Conflicts: # gradle/libs.versions.toml
|
Fixed now in Root cause was Android public Paykit publishing only refreshed the reusable on-chain address if the cached address was already reserved/unavailable. In the delete profile -> recreate profile flow, Lightning receive could be unavailable and the cached reusable on-chain address could still be blank, so endpoint sync concluded there were no supported endpoints and showed the toast. I changed public Paykit endpoint sync to ensure a reusable on-chain address exists before deciding there is no publishable endpoint, and added regression coverage for the blank-address case. Also merged latest Checked:
GitHub now reports the PR as mergeable. |
|
Manual regression — Paykit / contact paymentsEnvironment: regtest, staging Test setup
Session 1 — fresh profiles (smoke)
Session 1 looked good for basic contact + payment flows cross-platform. Private Paykit in session 1: Incoming activity showing “Received from [contact]” indicates the receive path worked — that label is only set when the payment matches a private Paykit invoice/address (not a generic public profile invoice). There are no private Paykit link errors in session 1 logs on either platform. Send-side logs showing Session 2 — profile delete, re-create, re-add contacts, second delete blocked
Delete profile: Screen.Recording.2026-07-01.at.15.18.17.movRegression — private Paykit broken after profile resetSession 1: Private Paykit appears to work (receive-side “Received from contact” + no link errors). Later in the same session, a second profile delete also failed on both platforms — private Paykit cleanup runs before delete and throws Private Paykit errors (identical on both platforms)Every private Paykit attempt ( First failures appear immediately after profile re-create (~12:23 iOS, ~12:26 Android on contact re-add). Contact payments fall back to publicPayments use a public BIP21 unified invoice from the contact’s published profile — not an encrypted private payment list:
Second profile delete blockedProfile delete runs private Paykit endpoint cleanup first. With private Paykit already broken, cleanup throws Android ( iOS ( Profile reset sequence (both sides)
Likely cause: local Paykit SDK state is wiped on delete/re-create, but encrypted-link handshake state is inconsistent across peers. SDK reports recovery is required; the app logs warnings, skips private publish, and resolves contact payments via public endpoints (intentional fallback). Useful grep patterns: Verdict
Not approving on “private contact payments survive profile delete/re-add.” Session 1 private Paykit looks fine; session 2 regresses on private Paykit recovery and blocks a second profile delete. |
|
Fixed in Main thing is we now use Paykit I also fixed the related app-side edges:
Public fallback while private recovery/link work is unavailable is still intentional so contact payments can still complete. Ring is still public-only for now; this fixes the crash path, not full Ring private payments support. |
|
@ben-kaufman is pubky-ring option disabled? |
|
@piotr-iohk Added it back for now, but we will likely remove it, still waiting for final decision on that... |
OK, atm clicking at Import with Pubky ring results in error toast. Not sure then if we want to resolve that or just leave for now? that is on both iOS and Android Screen.Recording.2026-07-03.at.12.44.46.mov |
Manual regression retest (Jul 3, post rc23)Environment: regtest, staging Logs:
Same flow as Jul 1: create profiles → add contacts → LN + on-chain (verify private) → delete → re-create (same pubky) → re-add → LN + on-chain → delete again. Results
Session 2 — private Paykit still broken after profile resetAfter delete/re-create/re-add, private link fails again on both platforms: Contact payments still complete via public fallback (by design). On Android, post-reset sends resolve to public BIP21 First failures after re-add: ~10:52 Android, ~10:52 UTC iOS. Fixed since Jul 1 — profile delete no longer blockedSecond delete succeeds even when private cleanup fails. Logs show Verdict
Not approving on “private contact payments survive profile delete/re-add.” Happy to re-test after another SDK/app fix; delete trap fix looks good. Useful grep patterns: |
|
Fixed and tested now. I reran the Android rc26 E2E with two fresh dev installs: Bitkit profiles on both sides, Pay Contacts enabled, contacts added/resolved both ways, Alice paid Bob from Send -> Contact, and Bob's received activity was assigned to Alice with the contact chip + Detach action. I also checked the app logs/DB for the run: no no-endpoint/public-fallback/private-unavailable/send-failure markers, and both latest activity rows have the expected contact keys. |
Manual regression retest (Jul 7)Environment: regtest, staging Logs:
Cross-platform pair: Android ↔ iOS sim. Same flow as prior retests (Jul 1 / Jul 3) plus PR QA checklist from #1040. PR QA checklist
Session flow (regression focus)
Jul 3 blockers — status in this run:
Log support: multiple Known issue — deferred (Android only)Pubky Ring profile import on Android fails after Ring returns auth success: 
UI: “Authorization Failed” toast on Join the Pubky Web screen (Import with Pubky Ring). iOS: Ring import works ( Agreed with @ben-kaufman on Slack to merge without blocking on this — Android Ring import tracked as follow-up, not a Paykit SDK regression. Verdict
LGTM on #1040 / #606 for merge, modulo deferred Android Ring import. |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approved except for one comment that worth addressing
5 participants
This PR:
com.synonym:paykit-android:0.1.0-rc23artifact.Description
Preview
N/A - SDK integration; no UI layout change.
QA Notes
Manual Tests
Automated Checks
./gradlew compileDevDebugKotlinpassed../gradlew testDevDebugUnitTestpassed../gradlew testDevDebugUnitTest --tests to.bitkit.repositories.PrivatePaykitRepoTestpassed../gradlew detektpassed.git diff --checkpassed.